Reject HTTP redirects on filter list and resource downloads#5621
Reject HTTP redirects on filter list and resource downloads#5621chrmod wants to merge 2 commits into
Conversation
Protects against redirect-based attacks where a compromised origin (GitHub raw, cdn.ghostery.com) could redirect to an attacker-controlled host serving malicious filter lists. Fetch and axios both follow redirects by default, so a 3xx response was silently accepted. Also replaces axios with native fetch in the stress-test tool to drop an extra dependency.
seia-soto
left a comment
seia-soto
left a comment
There was a problem hiding this comment.
I don't know if avoiding redirect at all without a flag would be useful because many lists from GitHub may contain redirect unintentionally (like to raw.github...). There should be more similar cases as well. There's a pattern that people often try to do higher level processing in path level levering CDNs.
Migrating from axios to fetch, I wouldn't mind using both despite the recent accident.
| export type Fetch = (url: string) => Promise<FetchResponse>; | ||
| interface FetchInit { | ||
| redirect?: 'error' | 'follow' | 'manual'; | ||
| } | ||
|
|
||
| export type Fetch = (url: string, init?: FetchInit) => Promise<FetchResponse>; |
There was a problem hiding this comment.
It should be "RequestInit" from built-in.
|
Not following redirects of arbitrary resources blindly makes sense, but here the Ghostery Adblocker is download from Ghostery infrastructure. If an attacker can control it, they could directly serve malicious content. (There is not much to do in that scenario. The best that I could see would be to sign the resources and hope that attackers are not also able to fake those signatures.) In other words, there is no compelling attack vector IMHO that justifies that we prevent redirects. A side-effect in the PR is to replace Axios by native fetch. I do not mind, but it is a different topic. I would recommend to open a new one for that. |
3 participants
Summary
Harden the adblocker against redirect-based supply-chain attacks. Until now, every remote resource fetch followed HTTP redirects by default, which meant a compromised or hijacked origin (GitHub raw content,
cdn.ghostery.com) could quietly redirect clients to an attacker-controlled host serving malicious filter lists.Fetchtype with an optionalinitargument and havefetchWithRetrypass{ redirect: 'error' }, so all consumers going throughFiltersEngine.fromLists/fromPrebuiltAds*now reject 3xx responses instead of silently following them.{ redirect: 'error' }option to the Node fetch calls in the dailyassets/update.jsresource-refresh script.axioswith nativefetchintools/stress-test-engine-update.ts(and drop theaxiosdevDependency), again rejecting redirects explicitly.The
Fetchtype change is backwards-compatible — callers passingglobalThis.fetchor a narrower one-argument function remain assignable.Test plan
FiltersEngine.fromPrebuiltAdsAndTracking(fetch)against the real CDN and confirm the engine still initializes successfullyassets/update.jsand confirm resources are refreshed without errors